Fix DrawOnlyWidget rendering problems - #48
Conversation
|
This causes conflicts for now... |
|
Solved this with an agent so i will paste its report: Rebased onto Two things changed since the original patch:
The CI is green on Go 1.25/1.26/1.27 for Linux, macOS and Windows. |
|
I found a layout regression in the new The flag remains true inside a A minimal reproduction is this structure: ctx.SetGridLayout(nil, []int{300})
ctx.GridCell(func(image.Rectangle) {
ctx.SetGridLayout(nil, []int{250})
ctx.Panel(func(layout debugui.ContainerLayout) {
ctx.SetGridLayout(nil, []int{150})
ctx.DrawOnlyWidget(func(*ebiten.Image) {})
ctx.GridCell(func(bounds image.Rectangle) {
// Inspect bounds.Min.Y - layout.BodyBounds.Min.Y.
})
})
})I verified that the following cell's vertical offset is 159 pixels on the base and only 5 pixels with this PR. The same panel outside the enclosing Could the flag be scoped to the layout that owns the callback, or saved/reset/restored when entering an independent container layout? That should preserve the scrolling fix without changing layout allocation inside nested panels. Review comment authored by Codex (OpenAI), on behalf of @hajimehoshi. |
|
Following up on the nested-panel regression: I would favor preserving The callback can draw beyond the row allocated by func (c *Context) DrawOnlyWidget(f func(screen *ebiten.Image)) {
_ = c.wrapEventHandlerAndError(func() (EventHandler, error) {
if _, err := c.layoutNext(); err != nil {
return nil, err
}
c.setClip(c.clipRect())
defer c.setClip(unclippedRect)
cmd := c.appendCommand(commandDraw)
cmd.draw.f = f
return nil, nil
})
}This would keep row allocation consistent for standalone calls and calls inside nested panels, while allowing drawing in a partially visible grid cell to continue. It also avoids making layout allocation depend on whether an ancestor is currently running a layout callback. No change to other widgets' visibility checks is needed for this approach. The tradeoff is that offscreen callbacks would still execute. The proposed This is a suggested alternative, not a tested patch. I would validate it against both the scrolling regression and the nested-panel layout regression described above. Comment authored by Codex (OpenAI), on behalf of @hajimehoshi. |
9b81555 to
7059efa
Compare
|
The simplified implementation looks good. Could you remove the two added tests and the The command-count test exposes internal drawing bookkeeping and requires exactly five queued commands, without verifying the visible rendering. The nested-panel test uses public behavior, but it primarily guards against the now-removed Please also fix the import grouping in Comment authored by Codex (OpenAI), on behalf of @hajimehoshi. |
DrawOnlyWidget created a nested widget, and that widget was culled when the bounds of its layout item did not overlap the container body. The callback can draw outside those bounds, so they cannot tell whether the output is visible: in a grid cell the item has the default height at the top of the cell, and the drawing disappeared once that item scrolled out while the cell was still visible. Allocate the layout item as before, so that the layout does not depend on whether the drawing is visible, and add the draw command without the visibility check. The clip rect already limits the output to the visible area. Also replace the deprecated vector.DrawFilledRect with vector.FillRect. Fixes ebitengine#47
DrawOnlyWidget created a nested widget, and that widget was culled when the bounds of its layout item did not overlap the container body. The callback can draw outside those bounds, so they cannot tell whether the output is visible: in a grid cell the item has the default height at the top of the cell, and the drawing disappeared once that item scrolled out while the cell was still visible. Allocate the layout item as before, so that the layout does not depend on whether the drawing is visible, and add the draw command without the visibility check. The clip rect already limits the output to the visible area. Also replace the deprecated vector.DrawFilledRect with vector.FillRect. Closes #47
Fixes #47
The bug
DrawOnlyWidgetcreated a nested widget, andwidgetskipped the drawing when the bounds of thatwidget's layout item did not overlap the container body. Those bounds cannot tell whether the
output is visible, because the callback can draw outside them: in a grid cell the item is 18px
tall at the top of the cell, so the drawing disappeared as soon as that item scrolled out of the
window body, even though the cell was still visible.
The fix
Keep allocating the layout item, so that the layout does not depend on whether the drawing is
visible, and add the draw command without the bounds check. The clip rect already limits the output
to the visible area, and offscreen callbacks running is the accepted tradeoff for not having
explicit bounds.
layout.goewidget.goficam intocados.Also in this PR:
vector.DrawFilledRectis replaced withvector.FillRect(deprecated sinceEbitengine v2.9).